Skip to content

Fix PlutusV4 script handling and scrambled ToPlutusScriptPurpose type family#1237

Merged
carbolymer merged 1 commit into
masterfrom
mgalazyn/fix/fix-plutus-v4-bugs
Jun 24, 2026
Merged

Fix PlutusV4 script handling and scrambled ToPlutusScriptPurpose type family#1237
carbolymer merged 1 commit into
masterfrom
mgalazyn/fix/fix-plutus-v4-bugs

Conversation

@carbolymer

@carbolymer carbolymer commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Context

Several PlutusV4/Dijkstra-era bugs in script conversion functions:

  • fromShelleyBasedScript mislabelled DijkstraPlutusV4 as V3, causing script hash mismatches and MissingScript errors for V4 reference scripts
  • toShelleyScript crashed with error for V4 scripts
  • fromAlonzoLanguage mapped PlutusV4 to PlutusScriptV3
  • getPlutusDatum crashed with error for V4 spending scripts
  • ToPlutusScriptPurpose type family had CertItem/MintItem and VoterItem/ProposalItem swapped (introduced scrambled in Introduce new witness api #763, never caught because only the TxInItem branch is currently evaluated)

How to trust this PR

Each fix is mechanical - correcting a wrong constructor or removing an error stub.
The ToPlutusScriptPurpose fix can be verified by comparing to PlutusScriptFor in the same file (always had the correct mapping).

All cardano-api tests pass (nix build .#checks.x86_64-linux.cardano-api:test:cardano-api-test).

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated
  • Self-reviewed the diff
  • Changelog fragment added in .changes/

Copilot AI review requested due to automatic review settings June 19, 2026 14:16
@carbolymer carbolymer self-assigned this Jun 19, 2026
@carbolymer carbolymer force-pushed the mgalazyn/fix/fix-plutus-v4-bugs branch from 1992c85 to 9d97ebb Compare June 19, 2026 14:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR corrects Plutus V4 handling across the API by properly mapping ledger PlutusV4 to PlutusScriptV4, implementing V4 script conversion for the Dijkstra era, and fixing a scrambled ToPlutusScriptPurpose type family used by the experimental legacy-script shim.

Changes:

  • Fix fromAlonzoLanguage to map PlutusV4PlutusScriptV4 and implement toShelleyScript / fromShelleyBasedScript support for Plutus V4 in Dijkstra.
  • Remove an unconditional runtime error for V4 datums in experimental witness handling (getPlutusDatum).
  • Correct the purpose mapping in ToPlutusScriptPurpose (Cert/Mint and Voter/Proposal were swapped).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
cardano-api/src/Cardano/Api/Plutus/Internal/Script.hs Fixes Plutus V4 version mapping and adds Dijkstra-era V4 script conversion.
cardano-api/src/Cardano/Api/Experimental/Tx/Internal/AnyWitness.hs Enables datum extraction for Plutus V4 instead of erroring.
cardano-api/src/Cardano/Api/Experimental/Plutus/Internal/Shim/LegacyScripts.hs Fixes the ToPlutusScriptPurpose type family mapping for legacy witness conversion.

Comment thread cardano-api/src/Cardano/Api/Plutus/Internal/Script.hs
@carbolymer carbolymer force-pushed the mgalazyn/fix/fix-plutus-v4-bugs branch 2 times, most recently from 929bbfd to eab94e1 Compare June 19, 2026 14:30
ToPlutusScriptPurpose VoterItem = ProposingScript
ToPlutusScriptPurpose ProposalItem = VotingScript
ToPlutusScriptPurpose VoterItem = VotingScript
ToPlutusScriptPurpose ProposalItem = ProposingScript

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm 99% sure that this was not right.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread cardano-api/src/Cardano/Api/Experimental/Plutus/Internal/Shim/LegacyScripts.hs Outdated
Comment thread cardano-api/src/Cardano/Api/Plutus/Internal/Script.hs

@Jimbo4350 Jimbo4350 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment everything else LGTM


toPlutusScriptDatum
:: Witnessable TxInItem era
:: thing ~ TxInItem

@Jimbo4350 Jimbo4350 Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a cosmetic change. Please revert it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion about it, but it made me stop here for a moment and think why thing was set to TxInItem. I can revert it.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Comment thread cardano-api/test/cardano-api-test/Test/Cardano/Api/CBOR.hs
prop_roundtrip_GovernancePollAnswer_CBOR
]
<> [ testProperty
("decode only wrapped plutus script " <> show (pretty version) <> " CBOR")
@carbolymer carbolymer added this pull request to the merge queue Jun 24, 2026
Merged via the queue into master with commit 00616ab Jun 24, 2026
38 of 40 checks passed
@carbolymer carbolymer deleted the mgalazyn/fix/fix-plutus-v4-bugs branch June 24, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants